Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

evm: drop bailout option from validate_call_funds() #2652

Merged
merged 1 commit into from
Jan 13, 2025
Merged

Conversation

chfast
Copy link
Member

@chfast chfast commented Jan 13, 2025

The bailout option to validate_call_funds() makes it no-op function. Drop this option from the function so that the transaction validation code is not aware of the bailout semantic. Instead, ignore the validation result in the single place where it matters.

The bailout option to validate_call_funds() makes it no-op function.
Drop this option from the function so that the transaction validation
code is not aware of the bailout semantic. Instead, ignore the
validation result in the single place where it matters.
@chfast chfast requested a review from yperbasis January 13, 2025 14:01
if (const auto result = protocol::validate_call_funds(txn, evm, owned_funds, bailout);
result != ValidationResult::kOk) {
if (const auto result = protocol::validate_call_funds(txn, evm, owned_funds);
!bailout && result != ValidationResult::kOk) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The application is a big awkward because we call the function even if we don't need the result. But considering the bailout is not enabled often this somehow guarantees the validate_call_funds still work correctly without crashes or assert failures.

Copy link

codecov bot commented Jan 13, 2025

Codecov Report

Attention: Patch coverage is 68.75000% with 5 lines in your changes missing coverage. Please review.

Project coverage is 50.63%. Comparing base (fc6ac3f) to head (acb4dbc).
Report is 3 commits behind head on master.

Files with missing lines Patch % Lines
silkworm/core/protocol/validation.cpp 63.63% 2 Missing and 2 partials ⚠️
silkworm/core/execution/processor.cpp 66.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2652   +/-   ##
=======================================
  Coverage   50.63%   50.63%           
=======================================
  Files         784      784           
  Lines       52416    52416           
  Branches     8150     8150           
=======================================
  Hits        26541    26541           
  Misses      23553    23553           
  Partials     2322     2322           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@chfast chfast merged commit 0ed20d7 into master Jan 13, 2025
16 checks passed
@chfast chfast deleted the ci/rpc_bailout branch January 13, 2025 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants